-
-
Notifications
You must be signed in to change notification settings - Fork 451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft: Move to mongosh and support mongodb versions 4.4+ only #677
Conversation
0e76831
to
225383a
Compare
Tested with Ubuntu 20.04 aarch64 and 6.0.8 |
@tsgoff Thanks for testing. |
test> rs.status()
Puppet part: class { '::mongodb::globals':
manage_package_repo => true,
version => '6.0.0',
} ->
class { '::mongodb::client': } ->
class { '::mongodb::server':
ensure => present,
restart => false,
auth => true,
replset => "${replicaset}",
replset_config => {
"${replicaset}" => {
ensure => present,
members => $mongo_array
}
},
create_admin => true,
admin_username => 'admin',
admin_password => "${adminpw}",
admin_roles => ['root'],
dbpath => $dbpath,
storage_engine => 'wiredTiger',
bind_ip => ['0.0.0.0'],
store_creds => true,
keyfile => '/etc/mongo-key',
key => "${cluster_key}",
} Generated /etc/mongod.conf
|
Replicaset is not initiated. Since auth is enabled, but the admin user is not yet created (we need the replicaset initiated first to be able to do that), it ss only possible to connect to mongod over localhost. But we still try to connect over the IP connection. Rescue is working since we can only use rs.status() at this point. need to check the code why its not connecting over localhost. |
with older versions it worked to stop the daemon, create the admin user with mongo shell, set a lock file and restart mongodb with another puppet apply. old version without mongosh: https://raw.githubusercontent.com/tsgoff/mongodb-demo/master/.deployment/codedeploy/init.sh In my test case the admin user is created and mongosh can authenticate with the .mongosh.rc |
Current method is based on : https://www.mongodb.com/docs/v6.0/core/localhost-exception/#std-label-localhost-exception It is I think still possible to use the method you described, but that not sure if that shoudl be implemented in the providers. It works on Redhat with current code, so I'm a but surprised it doesn't on ubuntu. Still struggling to setup local aceptance test environment, so no ubuntu/debian available yet for testing the code properly. |
With auth -> false I get
/root/.mongoshrc.js
Mongosh command on disabled auth is only working without host, 127.0.0.1, localhost but not with actual hostname or host ip. |
Hello, Do you have any idea about this ? Thanks. |
6e3c95a
to
4c70421
Compare
5231e51
to
d411167
Compare
I've tested your branch for an existing 4.4 cluster using TLS with BasicAuth and later upgraded it to 5.0 and 6.0 on Ubuntu 20.04.
diff --git a/lib/puppet/provider/mongodb.rb b/lib/puppet/provider/mongodb.rb
index ec0ed3f..9cae1ea 100644
--- a/lib/puppet/provider/mongodb.rb
+++ b/lib/puppet/provider/mongodb.rb
@@ -111,7 +111,7 @@ class Puppet::Provider::Mongodb < Puppet::Provider
args.push('--tlsAllowInvalidHostnames') if tls_invalid_hostnames(config)
end
- if $config['auth_mechanism'] && $config['auth_mechanism'] == 'x509'
+ if config['auth_mechanism'] && config['auth_mechanism'] == 'x509'
args.push("--authenticationDatabase '$external' --authenticationMechanism MONGODB-X509")
end
diff --git a/lib/facter/is_master.rb b/lib/facter/is_master.rb
index a42c04d..c251710 100644
--- a/lib/facter/is_master.rb
+++ b/lib/facter/is_master.rb
@@ -42,6 +42,7 @@ def get_options_from_hash_config(config)
result << "--tls --host #{Facter.value(:fqdn)}" if config['net.tls.mode'] == 'requireTLS' || !tlscert.nil? || !config['net.tls.CAFile'].nil?
result << "--tlsCertificateKeyFile #{tlscert}" unless tlscert.nil?
result << "--tlsCAFile #{config['net.tls.CAFile']}" unless config['net.tls.CAFile'].nil?
+ result << "--tlsUseSystemCA" if config['net.tls.CAFile'].nil?
# use --authenticationMechanism, ---authenticationDatabase
# when
diff --git a/lib/puppet/provider/mongodb.rb b/lib/puppet/provider/mongodb.rb
index 9cae1ea..2bd5d9d 100644
--- a/lib/puppet/provider/mongodb.rb
+++ b/lib/puppet/provider/mongodb.rb
@@ -106,6 +106,7 @@ class Puppet::Provider::Mongodb < Puppet::Provider
tls_ca = config['tlsca']
args += ['--tlsCAFile', tls_ca] unless tls_ca.nil?
+ args += ['--tlsUseSystemCA'] if tls_ca.nil?
args += ['--tlsCertificateKeyFile', config['tlscert']]
args.push('--tlsAllowInvalidHostnames') if tls_invalid_hostnames(config) bonus, this one is cosmetic: you've added an additional empty line in |
@Wimmesberger Thank you ... |
* use x509 autentication method for admin user, used to login in the api. * add supported_athentication_mechanisms, whish restricts the suppported authentication mechanism supported by the server.
I was just browsing PRs in this module trying to figure out why it doesn't have stdlib 9 support yet. Wouldn't it be better to implement a second set of providers targeting mongosh instead of changing existing ones? |
Please note with MongoDB 7x you need to remove the diff --git a/modules/mongodb/manifests/server.pp b/modules/mongodb/manifests/server.pp
index fb11ee2b..4d4335d4 100644
--- a/modules/mongodb/manifests/server.pp
+++ b/modules/mongodb/manifests/server.pp
@@ -379,2 +379,3 @@ class mongodb::server (
Optional[Boolean] $nojournal = undef,
+ Optional[Boolean] $removejournal = undef,
Optional[Boolean] $smallfiles = undef,
diff --git a/modules/mongodb/manifests/server/config.pp b/modules/mongodb/manifests/server/config.pp
index 18e830bc..d7ed2420 100644
--- a/modules/mongodb/manifests/server/config.pp
+++ b/modules/mongodb/manifests/server/config.pp
@@ -21,2 +21,3 @@ class mongodb::server::config {
$nojournal = $mongodb::server::nojournal
+ $removejournal = $mongodb::server::removejournal
$smallfiles = $mongodb::server::smallfiles
diff --git a/modules/mongodb/templates/mongodb.conf.erb b/modules/mongodb/templates/mongodb.conf.erb
index 709e688b..320d233e 100644
--- a/modules/mongodb/templates/mongodb.conf.erb
+++ b/modules/mongodb/templates/mongodb.conf.erb
@@ -48,2 +48,5 @@ storage.dbPath: <%= @dbpath %>
storage.journal.enabled: false
+<% elsif @removejournal -%>
+#Removed in 6x as Mongo now always enables journaling.
+#storage.journal.enabled: true
<% elsif @journal -%> I comment out the directive for clarity but you'll probably just wish to remove it. If you don't you get this when attempting to start the server...
|
Also if you choose not to use client certs you don't want to pass the diff --git a/modules/mongodb7plus/lib/puppet/provider/mongodb.rb b/modules/mongodb7plus/lib/puppet/provider/mongodb.rb
index 7448ca20..84918c7f 100644
--- a/modules/mongodb7plus/lib/puppet/provider/mongodb.rb
+++ b/modules/mongodb7plus/lib/puppet/provider/mongodb.rb
@@ -107,7 +107,7 @@ class Puppet::Provider::Mongodb < Puppet::Provider
tls_ca = config['tlsca']
args += ['--tlsCAFile', tls_ca] unless tls_ca.nil?
args += ['--tlsUseSystemCA'] if tls_ca.nil?
- args += ['--tlsCertificateKeyFile', config['tlscert']]
+ #args += ['--tlsCertificateKeyFile', config['tlscert']] ^^ You probably want to check for |
Another minor issue...if you want the tlsUseSystemCA param to be added to the config then it needs to be available to the erb template via the config file... diff --git a/modules/mongodb/manifests/server/config.pp b/modules/mongodb/manifests/server/config.pp
index d7ed2420..b4ab03cc 100644
--- a/modules/mongodb/manifests/server/config.pp
+++ b/modules/mongodb/manifests/server/config.pp
@@ -77,4 +77,5 @@ class mongodb::server::config {
$tls_invalid_hostnames = $mongodb::server::tls_invalid_hostnames
$tls_mode = $mongodb::server::tls_mode
+ $tls_use_system_ca = $mongodb::server::tls_use_system_ca
$storage_engine = $mongodb::server::storage_engine |
Can we please resolve the following...
Can we not create the necessary admin credentials before the config file is updated with auth: true ? This is a chicken and egg situation which constantly causes problems. The only solution is to disable auth / run the module / create the databases / enable auth. |
This should be fixed in #703 which should be merged soon ... |
OK will check and test again but I merged the entire #677 PR so pretty sure I have it. Either way I'll provide a more detailed report after the next run thanks. |
@witjoh so I can see your new check in mongodb.rb for "requires_authentication" message in mongodb.rb....and this is itself in an IF statement for the is_master fact....but I never evaluate it due to the following...
And checking the code you are running a Facter::Util::Resolution for mongosh and mongod...but I have those...
Any ideas why it's not "resolving" those binaries ? |
@whiphubley I took over work on this from @witjoh. Also, thank you for testing and your insights, I learned a lot from your comments. Once the migration to mongosh is complete, I'll add in proper support for MongoDB 6 and maybe 7. |
handled by #703. this can be closed now since mentioned PR is eady to be merged. |
This is a breaking PR